Repository navigation
feat(client): pass through sampling-mask logprobs and by-handle payload segments - #170
faresobeid wants to merge 3 commits into
Conversation
faresobeid
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The shown code change is a small, backward-compatible pass-through of optional server metadata and adds no request-side or security-sensitive behavior. Unresolved review comments nevertheless identify concrete reasoning-output parsing defects in the PR head, so human review is warranted. Notes:
You can add or adjust custom eligibility rules. Learn more. |
01c9cce to
1cd3446
Compare
e134d56 to
e161362
Compare
1cd3446 to
ea01237
Compare
e161362 to
19c558f
Compare
ea01237 to
ecfeef7
Compare
Dismissing prior approval to re-evaluate d3a1af0
faresobeid
changed the title
Dismissing prior approval to re-evaluate 030b208
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 030b208. Configure here.
| close == -1, text[start : close if close != -1 else len(text)], by_tool | ||
| close == -1, | ||
| text[start : close if close != -1 else len(text)].replace(open_marker, ""), | ||
| by_tool, |
There was a problem hiding this comment.
Reasoning text drops think markers
Medium Severity
scan_reasoning already skips the leading <think> opener, then deletes every later open_id and every <think> substring before decoding what remains. Reasoning that mentions the tag, or a truncated </think>, loses those characters, and neighboring tokens can merge into different text.
Reviewed by Cursor Bugbot for commit 030b208. Configure here.
| (reasoning or "").rstrip() | ||
| if parsed.boundary.is_open | ||
| else (reasoning or "").rstrip() or None | ||
| ), |
There was a problem hiding this comment.
DeepSeek reasoning boundary whitespace stripped
Medium Severity
parse_deepseek_v4 always rstrips reasoning and turns whitespace-only content into "". The newline the model emits before </think>, and a content channel that is only that separator, disappear from the parsed message.
Reviewed by Cursor Bugbot for commit 030b208. Configure here.
This branch has not been deployed
1 participant
On top of #174's packed completion logprobs,
renderers.client.generate()passes two more optional prime-rl/inference/v1/generatefields through. Both are pure pass-through: no request-side change, no parsing or validation (same assampling_mask). Stock vLLM sends neither, and then the keys areNone.sampling_mask_logprobs = choice.get("sampling_mask_logprobs"): the sampler's logprobs at the sampling-mask ids, packed base64 float32{data, shape, dtype}, parallel to the packedsampling_maskids. verifiers decodes it (feat(v1): optional sampler logprobs on SamplingMask verifiers#2756). Consumer: prime-rl score centering, feat: exact score centering over the sampling mask (arXiv:2609.20807) prime-rl#3794.payload = choice.get("payload"): by-handle segments of a shared file that prime-rl's inference server writes instead of returningrouted_experts/sampling_maskinline. verifiers puts them on traces (feat(v1): by-handle payload segments on traces verifiers#2765). Consumer: feat: pass router-replay ids and sampling masks by handle on multi-node runs prime-rl#3902.Stack
Base: #174 (
perf/packed-completion-logprobs). Order: #174 -> PrimeIntellect-ai/verifiers#2778 -> prime-rl#3900; then this PR with PrimeIntellect-ai/verifiers#2756 (-> prime-rl#3794) and PrimeIntellect-ai/verifiers#2765 (-> prime-rl#3902).Validation
pytest tests/test_client.py --noconftest(32 passed),ruff check,ruff format --checkonrenderers/client.py.Note
Add
sampling_mask_logprobsandpayloadtogenerateclient resultThe async generation client in client.py now reads two new fields from the generation choice and returns them.
sampling_mask_logprobscarries the server's renormalized sampling-mask logprobs (prime-rl may send these with packed sampling-mask data).payloadcarries by-handle side-array data. Both keys areNonewhen the choice does not provide the field.Macroscope summarized 030b208.
Note
Medium Risk
Refactors GLM and DeepSeek V4 completion parsing and multi-turn bridging—high-impact for tool-call finish reasons and RL trajectories—while client fields are pass-through only.
Overview
Inference client now forwards optional prime-rl choice fields
sampling_mask_logprobs(renormalized logprobs parallel to kept vocab ids) andpayload(by-handle segments for large side arrays) throughgenerate()’s result dict, alongside updated return-value docs.Tagged completion parsing adds
renderers/tagged.pywithscan_tagged_responseand separate GLM vs DeepSeek V4 state-machine grammars. GLM and DeepSeek V4 parsers inparsing.pyare rewritten on top of it (per-block tool conversion,thinking_enabled,reasoning_complete).scan_reasoningtreats thinking openers after leading whitespace and keeps literal delimiter spellings inside content when they are ordinary tokens, not atomic specials.GLM / DeepSeek V4 renderers drive parsing from
enable_thinkingwhenprompt_idsis empty (documented onparse_response), andbridge_to_next_turnusesparse_response+reasoning_completeinstead of ad-hocscan_reasoning. DeepSeek DSML parsing no longer depends on a separatedsml_idtoken scan path.Tests replace render→parse round-trips with reference-template continuations replayed through the real
generate()HTTP client (replay.py,reference_continuations.py), add GLM/DeepSeek tagged rollout integration tests, tighten reasoning-boundary expectations for formats that reopen reasoning, and add module-scoped cache/GC teardown to cut HF memory use in CI.Reviewed by Cursor Bugbot for commit 030b208. Bugbot is set up for automated code reviews on this repo. Configure here.